Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Call mavgen from command line #838

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hamishwillee
Copy link
Contributor

This allows you to call mavgen directly from the command line from anywhere after installing Pymavlink.

note that I don't "know" if this is optimal, or if you have to add pymavlink.tools to the packages:

image

Fixes #813

@julianoes For discussion/testing.

@rotu
Copy link
Contributor

rotu commented Jul 6, 2023

Your code is set up to call a function __main__ in the module pymavlink.tools.mavgen. So it imports the pymavlink.tools.mavgen module (which has the side effect of running its code) and then crashes.

mavgen .\message_definitions\v1.0\matrixpilot.xml

Validating .\message_definitions\v1.0\matrixpilot.xml
Parsing .\message_definitions\v1.0\matrixpilot.xml
Validating C:\Users\dan\Source\mavlink\message_definitions\v1.0\common.xml
Parsing C:\Users\dan\Source\mavlink\message_definitions\v1.0\common.xml
Validating C:\Users\dan\Source\mavlink\message_definitions\v1.0\standard.xml
Parsing C:\Users\dan\Source\mavlink\message_definitions\v1.0\standard.xml
Validating C:\Users\dan\Source\mavlink\message_definitions\v1.0\minimal.xml
Parsing C:\Users\dan\Source\mavlink\message_definitions\v1.0\minimal.xml
Merged enum MAV_CMD
Found 169 MAVLink message types in 4 XML files
Generating mavlink.py
Generating preamble
Generating enums
Generating message IDs
Generating class definitions
Generating MAVLink class
Generating methods
Generated mavlink.py OK
Traceback (most recent call last):
  File "\\?\C:\Users\dan\Source\mavlink\venv\Scripts\mavgen-script.py", line 33, in <module>
    sys.exit(load_entry_point('pymavlink==2.4.39', 'console_scripts', 'mavgen')())
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "\\?\C:\Users\dan\Source\mavlink\venv\Scripts\mavgen-script.py", line 25, in importlib_load_entry_point
    return next(matches).load()
           ^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\dan\.pyenv\pyenv-win\versions\3.11.4\Lib\importlib\metadata\__init__.py", line 204, in load
    return functools.reduce(getattr, attrs, module)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: module 'pymavlink.tools.mavgen' has no attribute '__main__'. Did you mean: '__name__'?

Also, can we get rid of mavgen.py (which doesn't work on Windows anyway) and the shebang (which implies mavgen.py can directly be called as a shell script)?

@hamishwillee
Copy link
Contributor Author

@rotu If I knew what I was doing we wouldn't have these problems. I just ran mavgen to the help stage. I'm happy for someone who knows what they are doing to take this on.

Mavgen.py doesn't work on Windows? I'm not sure what you mean? I run mavgen on Windows.

@rotu
Copy link
Contributor

rotu commented Jul 7, 2023

If I knew what I was doing we wouldn't have these problems.

Ha that’s such a familiar feeling to me…

Mavgen.py doesn't work on Windows? I'm not sure what you mean? I run mavgen on Windows.

How did you install Python and how do you invoke mavgen.py? There’s some vagaries (like whether you have a py.exe installed and the OS-level association of *.py files, since Windows doesn’t understand shebangs). Notably in Windows 11, if you type python at the command prompt, I believe it will open up the Windows Store to install a version of python without the py.exe launcher.

The console scripts endpoint creates an actual .exe file which ensures the code is run under the correct Python interpreter.

@hamishwillee
Copy link
Contributor Author

hamishwillee commented Jul 12, 2023

@rotu I must have been testing this in a ubuntu VM because as you say, it is not working on Windows - mavgen command is not being found/recognised.

Also, can we get rid of mavgen.py (which doesn't work on Windows anyway) and the shebang (which implies mavgen.py can directly be called as a shell script)?

No, the reason we're doing this is to allow mavgen.py to be called from anywhere following installation of pymavlink. That would allow us to build headers from Pymavlink rather than importing Pymavlink as a submodule.

@rotu
Copy link
Contributor

rotu commented Jul 12, 2023

No, the reason we're doing this is to allow mavgen.py to be called from anywhere following installation of pymavlink.

That's what the new console_scripts entrypoint is for - it can totally supplant the existing scripts entry. (Except without the .py suffix which is not needed. Heck, you could name the entrypoint mavlink.py for backwards compatibility, but I think that's a tad silly)

That would allow us to build headers from Pymavlink rather than importing Pymavlink as a submodule.

Currently all the build scripts call it via python /path/to/pymavlink/tools/mavgen.py (which does not rely on the scripts entry but does rely on the repo being checked out to a folder called "pymavlink"). Removing the scripts entry and the shebang won't affect this.

@hamishwillee
Copy link
Contributor Author

@rotu I wasn't joking when I said I don't know what I'm doing. I'm not sure how to progress from here to a console_script that does what is needed.

Going to have to wait until someone with more Python knowledge or more time shows up.

@julianoes
Copy link
Contributor

@peterbarker any objections against this? Does it break anything?

@hamishwillee
Copy link
Contributor Author

hamishwillee commented Sep 12, 2023

@julianoes This worked for me on Ubuntu but not on Windows. You should verify that it works for you on the platforms you need.

I'd be somewhat OK with including this for your case, except that I worry we'd then get bugs against Windows etc.

The approach should be something that can be turned into a working case though.
Based on my understanding of #838 (comment) - it builds an OS specific exe so you should be able to feed the console what you want. I just don't have knowledge or time to fiddle with it to get this working on other platforms (not when someone who knows what they are doing could solve it without having to do the 10000 monkeys for 10000 years thing)

I posted a request for help here too https://discord.com/channels/1022170275984457759/1151306036582551592

PS it should not break anything AFAIK.

@julianoes
Copy link
Contributor

@hamishwillee alright. I'll try it on Windows.

@peterbarker
Copy link
Contributor

Ping @julianoes - how did your testing go?

@julianoes
Copy link
Contributor

Thanks for the reminder @peterbarker.

@hamishwillee how do you use/test this on Windows? I've never done Python on Windows...

@hamishwillee
Copy link
Contributor Author

I can't remember. I''ll have to try this again in two weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to call mavgen?
4 participants